Some fixes to the magic import system #349
Merged
+48
−65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Stop adding the directory of every .capnp file to the import path. If a .capnp file wants to import a file in its own directory, it should use a relative import. Fixes capnproto#278 - Stop using /usr/include/capnp as an import path. This is incorrect. It should only be /usr/include. - Stop allowing additional paths to be specified for magic imports. This leads to inconsistencies. More specifically, the way that a nested import like `ma.mb.mc_capnp` gets imported by python, is to first import `ma`, then import `ma.mb`, and finally `ma.mb.mc_capnp`. Pycapnp's magic importing is only involved in the last step. So any additional paths specified don't work for nested imports. It is very confusing to only have this for non-nested imports. Users with folder layouts that don't follow pythons import paths can still use `capnp.load(.., .., imports=[blah])`.
LasseBlaauwbroek
force-pushed
the
fix-import
branch
from
d5e9b90
to
fd90196
Compare
|
Thanks for fixing this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rpc.capnpshould doimport capnp.rpc_capnpinstead ofimport rpc_capnp. This fixes another bug similar to Absolute import causes duplicate ID #278.ma.mb.mc_capnpgets imported by python, is to first importma, then importma.mb, and finallyma.mb.mc_capnp. Pycapnp's magic importing is only involved in the last step. So any additional paths specified don't work for nested imports. It is very confusing to only have this for non-nested imports. Users with folder layouts that don't follow pythons import paths can still usecapnp.load(.., .., imports=[blah]).Possibly this is also a fix for #279 but I cannot verify that.